This looks good to me.

Just to confirm I've understood correctly, you are currently leaving the agent-config.toml alone and not removing the AA_KBC param from there? I'm not completely sure of how the interaction between agent_config AA_KBC and the cdh config will be in CCv0, but you will understand better than I do the precedence of the new CDH configuration, so I guess that's okay for this PR, and we can re-visit when we switch to main

yes, we should remove that logic. I just pushed the draft PR in this state to get a libvirt test run since this is rather intrusive and I'd expect some breakage.

Compilation target (architecture, OS) tuple (s390x, linux) is not part of the supported tuples. Please compile with the "generate-bindings" feature or add support for your platform :)', /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tss-esapi-sys-0.5.0/build.rs:35:17

Ok, that error is unrelated to the change but it's another breaking change that prevents us from updating guest-components.. current revisions of guest-components will compile all attester modules and that doesn't always make sense e.g. the TDX bindings for the IBM archs.

I wonder why this didn't pop up on the guest-components CI, do we have s390x builds there?

I wonder why this didn't pop up on the guest-components CI, do we have s390x builds there?

I added some basic s390x workflow for CDH:
https://github.com/confidential-containers/guest-components/blob/18d8062899127302f691621e1c70bdcb4814a244/.github/workflows/cdh_basic.yml#L59-L61,

but maybe that isn't running the TEE_PLATFORM="all", or whatever the issue that Wainer also found meant there were libc incompatibilities. Maybe Ding might understand it better?

I wonder why this didn't pop up on the guest-components CI, do we have s390x builds there?

I added some basic s390x workflow for CDH: https://github.com/confidential-containers/guest-components/blob/18d8062899127302f691621e1c70bdcb4814a244/.github/workflows/cdh_basic.yml#L59-L61,

but maybe that isn't running the TEE_PLATFORM="all", or whatever the issue that Wainer also found meant there were libc incompatibilities. Maybe Ding might understand it better?

I see. we probably need something similar for attestation agent (which has the TEE specific attester code)

It should be possible to set the ATTESTER env explicitly to overwrite the all-attesters feature, let's see...

I fixed guest-components to a revision with CDH config file, but not the latest, which will not run anyway, due to problems with CCv0 (see #1750), so we don't run into the build issues either.

Just to confirm I've understood correctly, you are currently leaving the agent-config.toml alone and not removing the AA_KBC param from there? I'm not completely sure of how the interaction between agent_config AA_KBC and the cdh config will be in CCv0, but you will understand better than I do the precedence of the new CDH configuration, so I guess that's okay for this PR, and we can re-visit when we switch to main

After testing it became clear that we need both:

• AA_KBC_PARAMS in kata-agent to configure attestation-agent
• cdh.toml to configure confidential-data-hub

It has to do with passport model that was introduced. AA will perform remote-attestation w/ the configured KBS to get a token. CDH will use that token to retrieve a secret from KBS. In theory we could have different kbs'es for CDH and AA (token-kbs vs resource-kbs) but we don't have any config options for this.

After testing it became clear that we need both:

• AA_KBC_PARAMS in kata-agent to configure attestation-agent
• cdh.toml to configure confidential-data-hub

Thanks for the info.

Are there any plans to use an aa.toml type approach to avoid going through the kata-agent?

I guess that shows that we still need:
https://github.com/kata-containers/kata-containers/blob/c25983202cc7a45a03582edae2945c8807c1f9de/tests/integration/kubernetes/k8s-confidential-attestation.bats#L56-L59:

        # TODO - do we still need AA_KBC set and checked - do we have any other options?
	if [ "${AA_KBC}" = "cc_kbc" ]; then
		kernel_params_value+=" agent.aa_kbc_params=cc_kbc::${CC_KBS_ADDR}"
	fi

in the current merge-to-main attestation resource testing as well? Though I don't think we have AA_KBC set in out CI anywhere anymore, so maybe that is why kata-containers/kata-containers#8870 is failing?

Thanks for the info.

Are there any plans to use an aa.toml type approach to avoid going through the kata-agent?

I guess that shows that we still need: https://github.com/kata-containers/kata-containers/blob/c25983202cc7a45a03582edae2945c8807c1f9de/tests/integration/kubernetes/k8s-confidential-attestation.bats#L56-L59:

        # TODO - do we still need AA_KBC set and checked - do we have any other options?
	if [ "${AA_KBC}" = "cc_kbc" ]; then
		kernel_params_value+=" agent.aa_kbc_params=cc_kbc::${CC_KBS_ADDR}"
	fi

in the current merge-to-main attestation resource testing as well? Though I don't think we have AA_KBC set in out CI anywhere anymore, so maybe that is why kata-containers/kata-containers#8870 is failing?

there is some config file support for AA (see this PR), but I think it's not a full replacement for aa_kbc_params yet, since it only covers the uri part. So yes, I'd assume you still need to set aa_kbc_params and hence AA_KBC (or tweak the build script).